Skip to content

Conversation

@hymm
Copy link
Contributor

@hymm hymm commented Aug 29, 2023

Objective

  • Some of the old ambiguity tests didn't get ported over during schedule v3.

Solution

  • Port over tests from
    #[cfg(test)]
    mod tests {
    // Required to make the derive macro behave
    use crate as bevy_ecs;
    use crate::event::Events;
    use crate::prelude::*;
    #[derive(Resource)]
    struct R;
    #[derive(Component)]
    struct A;
    #[derive(Component)]
    struct B;
    // An event type
    struct E;
    fn empty_system() {}
    fn res_system(_res: Res<R>) {}
    fn resmut_system(_res: ResMut<R>) {}
    fn nonsend_system(_ns: NonSend<R>) {}
    fn nonsendmut_system(_ns: NonSendMut<R>) {}
    fn read_component_system(_query: Query<&A>) {}
    fn write_component_system(_query: Query<&mut A>) {}
    fn with_filtered_component_system(_query: Query<&mut A, With<B>>) {}
    fn without_filtered_component_system(_query: Query<&mut A, Without<B>>) {}
    fn event_reader_system(_reader: EventReader<E>) {}
    fn event_writer_system(_writer: EventWriter<E>) {}
    fn event_resource_system(_events: ResMut<Events<E>>) {}
    fn read_world_system(_world: &World) {}
    fn write_world_system(_world: &mut World) {}
    // Tests for conflict detection
    #[test]
    fn one_of_everything() {
    let mut world = World::new();
    world.insert_resource(R);
    world.spawn(A);
    world.init_resource::<Events<E>>();
    let mut test_stage = SystemStage::parallel();
    test_stage
    // nonsendmut system deliberately conflicts with resmut system
    .add_system(resmut_system)
    .add_system(write_component_system)
    .add_system(event_writer_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 0);
    }
    #[test]
    fn read_only() {
    let mut world = World::new();
    world.insert_resource(R);
    world.insert_non_send_resource(R);
    world.spawn(A);
    world.init_resource::<Events<E>>();
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(empty_system)
    .add_system(empty_system)
    .add_system(res_system)
    .add_system(res_system)
    .add_system(nonsend_system)
    .add_system(nonsend_system)
    .add_system(read_component_system)
    .add_system(read_component_system)
    .add_system(event_reader_system)
    .add_system(event_reader_system)
    .add_system(read_world_system)
    .add_system(read_world_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 0);
    }
    #[test]
    fn read_world() {
    let mut world = World::new();
    world.insert_resource(R);
    world.spawn(A);
    world.init_resource::<Events<E>>();
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(resmut_system)
    .add_system(write_component_system)
    .add_system(event_writer_system)
    .add_system(read_world_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 3);
    }
    #[test]
    fn resources() {
    let mut world = World::new();
    world.insert_resource(R);
    let mut test_stage = SystemStage::parallel();
    test_stage.add_system(resmut_system).add_system(res_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 1);
    }
    #[test]
    fn nonsend() {
    let mut world = World::new();
    world.insert_resource(R);
    world.insert_non_send_resource(R);
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(nonsendmut_system)
    .add_system(nonsend_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 1);
    }
    #[test]
    fn components() {
    let mut world = World::new();
    world.spawn(A);
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(read_component_system)
    .add_system(write_component_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 1);
    }
    #[test]
    #[ignore = "Known failing but fix is non-trivial: https://github.com/bevyengine/bevy/issues/4381"]
    fn filtered_components() {
    let mut world = World::new();
    world.spawn(A);
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(with_filtered_component_system)
    .add_system(without_filtered_component_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 0);
    }
    #[test]
    fn events() {
    let mut world = World::new();
    world.init_resource::<Events<E>>();
    let mut test_stage = SystemStage::parallel();
    test_stage
    // All of these systems clash
    .add_system(event_reader_system)
    .add_system(event_writer_system)
    .add_system(event_resource_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 3);
    }
    #[test]
    fn exclusive() {
    let mut world = World::new();
    world.insert_resource(R);
    world.spawn(A);
    world.init_resource::<Events<E>>();
    let mut test_stage = SystemStage::parallel();
    test_stage
    // All 3 of these conflict with each other
    .add_system(write_world_system)
    .add_system(write_world_system.at_end())
    .add_system(res_system.at_start())
    // These do not, as they're in different segments of the stage
    .add_system(write_world_system.at_start())
    .add_system(write_world_system.before_commands());
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 3);
    }
    // Tests for silencing and resolving ambiguities
    #[test]
    fn before_and_after() {
    let mut world = World::new();
    world.init_resource::<Events<E>>();
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(event_reader_system.before(event_writer_system))
    .add_system(event_writer_system)
    .add_system(event_resource_system.after(event_writer_system));
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 0);
    }
    #[test]
    fn ignore_all_ambiguities() {
    let mut world = World::new();
    world.insert_resource(R);
    world.insert_non_send_resource(R);
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(resmut_system.ignore_all_ambiguities())
    .add_system(res_system)
    .add_system(nonsend_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 0);
    }
    #[test]
    fn ambiguous_with_label() {
    let mut world = World::new();
    world.insert_resource(R);
    world.insert_non_send_resource(R);
    #[derive(SystemLabel)]
    struct IgnoreMe;
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(resmut_system.ambiguous_with(IgnoreMe))
    .add_system(res_system.label(IgnoreMe))
    .add_system(nonsend_system.label(IgnoreMe));
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 0);
    }
    #[test]
    fn ambiguous_with_system() {
    let mut world = World::new();
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(write_component_system.ambiguous_with(read_component_system))
    .add_system(read_component_system);
    test_stage.run(&mut world);
    assert_eq!(test_stage.ambiguity_count(&world), 0);
    }
    fn system_a(_res: ResMut<R>) {}
    fn system_b(_res: ResMut<R>) {}
    fn system_c(_res: ResMut<R>) {}
    fn system_d(_res: ResMut<R>) {}
    fn system_e(_res: ResMut<R>) {}
    // Tests that the correct ambiguities were reported in the correct order.
    #[test]
    fn correct_ambiguities() {
    use super::*;
    let mut world = World::new();
    world.insert_resource(R);
    let mut test_stage = SystemStage::parallel();
    test_stage
    .add_system(system_a)
    .add_system(system_b)
    .add_system(system_c.ignore_all_ambiguities())
    .add_system(system_d.ambiguous_with(system_b))
    .add_system(system_e.after(system_a));
    test_stage.run(&mut world);
    let ambiguities = test_stage.ambiguities(&world);
    assert_eq!(
    ambiguities,
    vec![
    SystemOrderAmbiguity {
    system_names: [
    "bevy_ecs::schedule::ambiguity_detection::tests::system_a".to_string(),
    "bevy_ecs::schedule::ambiguity_detection::tests::system_b".to_string()
    ],
    conflicts: vec!["bevy_ecs::schedule::ambiguity_detection::tests::R".to_string()],
    segment: SystemStageSegment::Parallel,
    },
    SystemOrderAmbiguity {
    system_names: [
    "bevy_ecs::schedule::ambiguity_detection::tests::system_a".to_string(),
    "bevy_ecs::schedule::ambiguity_detection::tests::system_d".to_string()
    ],
    conflicts: vec!["bevy_ecs::schedule::ambiguity_detection::tests::R".to_string()],
    segment: SystemStageSegment::Parallel,
    },
    SystemOrderAmbiguity {
    system_names: [
    "bevy_ecs::schedule::ambiguity_detection::tests::system_b".to_string(),
    "bevy_ecs::schedule::ambiguity_detection::tests::system_e".to_string()
    ],
    conflicts: vec!["bevy_ecs::schedule::ambiguity_detection::tests::R".to_string()],
    segment: SystemStageSegment::Parallel,
    },
    SystemOrderAmbiguity {
    system_names: [
    "bevy_ecs::schedule::ambiguity_detection::tests::system_d".to_string(),
    "bevy_ecs::schedule::ambiguity_detection::tests::system_e".to_string()
    ],
    conflicts: vec!["bevy_ecs::schedule::ambiguity_detection::tests::R".to_string()],
    segment: SystemStageSegment::Parallel,
    },
    ]
    );
    }
    }
    with minimal changes
  • Make a method to convert the ambiguity conflicts to a string for easier verification of correct results.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change labels Aug 29, 2023
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to see these revived: thanks!

Copy link
Member

@joseph-gio joseph-gio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know these got deleted, thanks! They were added in 0.9 (#6053), so looks like they only survived for one release before being yoten haha.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 29, 2023
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Aug 29, 2023
Merged via the queue into bevyengine:main with commit da9a070 Aug 29, 2023
@hymm hymm deleted the port-old-ambiguity-tests branch August 30, 2023 02:04
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
# Objective

- Some of the old ambiguity tests didn't get ported over during schedule
v3.

## Solution

- Port over tests from
https://github.com/bevyengine/bevy/blob/15ee98db8d1c6705111e0f11a8fc240ceaf9f2db/crates/bevy_ecs/src/schedule/ambiguity_detection.rs#L279-L612
with minimal changes
- Make a method to convert the ambiguity conflicts to a string for
easier verification of correct results.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants